Speed up slow specs#22390
Conversation
There was a problem hiding this comment.
Pull request overview
This PR restructures a set of Homebrew specs to reduce runtime by avoiding repeated brew subprocesses and expensive external work (SVN, DMG mounting, signal waits), while preserving coverage via in-process command-object specs and a smaller number of targeted shell-path harnesses.
Changes:
- Convert many command specs from
brew ...subprocess integration tests to direct command-object (klass.new(...).run) tests where subprocess adds little signal. - Consolidate remaining Bash-path coverage into fewer subprocess harnesses (e.g.,
list,which-formula,--repository) and stub slow external work (SVN, DMG extraction/mounting, sleep). - Document the intended integration-test limit per command in
AGENTS.md.
Reviewed changes
Copilot reviewed 38 out of 38 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| Library/Homebrew/test/utils/svn_spec.rb | Stubs system_command results to test SVN availability/version logic without invoking real svn. |
| Library/Homebrew/test/unpack_strategy/dmg_spec.rb | Replaces shared extract example with a fully stubbed mount/extract path to avoid real DMG work. |
| Library/Homebrew/test/spec_helper.rb | Speeds up :needs_svn by caching skip reason/PATH dirs and minimizing repeated discovery. |
| Library/Homebrew/test/installed_dependents_spec.rb | Avoids Tab.create cost by writing a minimal Tab receipt directly for keg setup. |
| Library/Homebrew/test/download_strategies/subversion_spec.rb | Requires utils/svn and stubs SVN version to keep :trust_cert arg coverage fast. |
| Library/Homebrew/test/dev-cmd/unpack_spec.rb | Switches from brew unpack subprocess to direct Homebrew::DevCmd::Unpack execution. |
| Library/Homebrew/test/dev-cmd/ruby_spec.rb | Replaces slow subprocess test with an in-process expectation on exec arguments. |
| Library/Homebrew/test/dev-cmd/livecheck_spec.rb | Converts a usage/error-path integration test to in-process UsageError assertion. |
| Library/Homebrew/test/dev-cmd/linkage_spec.rb | Replaces integration check with a direct formula dependency tag assertion. |
| Library/Homebrew/test/dev-cmd/formula_spec.rb | Writes a minimal core tap formula file to avoid heavier setup while testing path output. |
| Library/Homebrew/test/dev-cmd/extract_spec.rb | Builds tap history by writing formula files directly, then runs extract via command object. |
| Library/Homebrew/test/dev-cmd/bump_spec.rb | Converts an error-path check to in-process UsageError assertion (stubbing bundler install). |
| Library/Homebrew/test/cmd/which-formula_spec.rb | Consolidates Bash-path coverage into one Open3.capture3 harness with multiple checks. |
| Library/Homebrew/test/cmd/uses_spec.rb | Stubs NamedArgs#to_formulae failure and intersections to avoid integration setup. |
| Library/Homebrew/test/cmd/upgrade_spec.rb | Consolidates formula+cask happy path and moves multiple branches to in-process command execution. |
| Library/Homebrew/test/cmd/untap_spec.rb | Replaces formula-file setup helper usage with direct tap file writes and cache clearing. |
| Library/Homebrew/test/cmd/unpin_spec.rb | Converts cask unpinning tests to in-process command execution. |
| Library/Homebrew/test/cmd/unalias_spec.rb | Sets up aliases in-process (via Homebrew::Aliases) while still testing brew unalias. |
| Library/Homebrew/test/cmd/tap-info_spec.rb | Replaces network-heavy integration test with stubbing Tap.installed and running command in-process. |
| Library/Homebrew/test/cmd/pin_spec.rb | Converts several pin paths (cask + error path) to in-process command execution with stubs. |
| Library/Homebrew/test/cmd/outdated_spec.rb | Moves --minimum-version logic to in-process tests and expands JSON coverage to include casks. |
| Library/Homebrew/test/cmd/list_spec.rb | Collapses multiple Bash-path checks into a single shell harness; converts some Ruby fallback paths to UsageError assertions. |
| Library/Homebrew/test/cmd/link_spec.rb | Replaces integration setup with fully stubbed keg/formula interactions to assert “no keg-only output”. |
| Library/Homebrew/test/cmd/leaves_spec.rb | Converts most cases to in-process with stubs, keeping one integration test where helpful. |
| Library/Homebrew/test/cmd/install_spec.rb | Consolidates bottle/source coverage into a combined integration scenario; keeps HEAD install coverage separate. |
| Library/Homebrew/test/cmd/info_spec.rb | Converts JSON v1 + --quiet checks to in-process execution with stubbed named args. |
| Library/Homebrew/test/cmd/home_spec.rb | Converts homepage-opening behavior to in-process exec_browser expectations for formula/cask/mixed args. |
| Library/Homebrew/test/cmd/fetch_spec.rb | Combines formula+cask fetch concurrency coverage into one integration test. |
| Library/Homebrew/test/cmd/desc_spec.rb | Converts --search API/error branches to in-process tests asserting UsageError / search calls. |
| Library/Homebrew/test/cmd/--repository_spec.rb | Replaces two subprocess tests with one consolidated Bash harness validating multiple arguments. |
| Library/Homebrew/test/cmd/--prefix_spec.rb | Converts formula resolution and error/warning branches to in-process tests with stubs. |
| Library/Homebrew/test/cmd/--cellar_spec.rb | Converts formula cellar path check to in-process test with stubbed resolved formula. |
| Library/Homebrew/test/cmd/--caskroom_spec.rb | Converts caskroom path printing for named casks to in-process test with stubbed casks. |
| Library/Homebrew/test/cmd/--cache_spec.rb | Converts cache-path checks to in-process execution for formula/cask cases. |
| Library/Homebrew/test/cask/upgrade_spec.rb | Stubs cask installation/upgrade to avoid real upgrades while still exercising multi-error behavior. |
| Library/Homebrew/test/cask/installer_spec.rb | Stubs DMG extraction path to avoid mounting/extracting real DMGs while testing installer logic. |
| Library/Homebrew/test/cask/artifact/shared_examples/uninstall_zap.rb | Stubs sleep(3) to avoid real waiting in signal-wait behavior. |
| AGENTS.md | Updates contributor guidance to prefer a single fast happy-path :integration_test per command. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
9cdbeb6 to
a0e331c
Compare
a0e331c to
79ea342
Compare
- Reduce slow command specs because repeated `brew` subprocesses were dominating the profile more than the tested Homebrew behaviour. - Preserve coverage by moving option and error branches into direct command-object specs where a full process adds no useful signal. - Keep shell-specific coverage by folding Bash command checks into one subprocess harness for each affected Bash path. - Combine formula and cask happy paths in `fetch`, `outdated` and `upgrade`, so one process still checks both package types. - Keep `cmd/install` source, bottle, keg-only and `--HEAD` coverage, with `--HEAD` separate because the option applies to the whole call. - Stub expensive `svn`, DMG mount and signal-wait paths so the specs cover Homebrew decisions without invoking slow external work. - Restore `brew extract` monkey patches when specs run in-process, so dependency and formula requirement caches do not leak across examples. - Document the integration-test limit in `AGENTS.md` so future command specs bias towards fast in-process coverage.
79ea342 to
85a29ba
Compare
|
Before:
After:
Not bad 😎 |
brewsubprocesses were dominating the profile more than the tested Homebrew behaviour.fetch,outdatedandupgrade, so one process still checks both package types.cmd/installsource, bottle, keg-only and--HEADcoverage, with--HEADseparate because the option applies to the whole call.svn, DMG mount and signal-wait paths so the specs cover Homebrew decisions without invoking slow external work.AGENTS.mdso future command specs bias towards fast in-process coverage.brew lgtm(style, typechecking and tests) with your changes locally?OpenAI Codex 5.5 xhigh with local review and testing.